Add atomic Resource demand observations - #404
Conversation
059287e to
95b915d
Compare
schickling-assistant
changed the title
|
@codex please review this PR. Posted on behalf of @schickling
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d072a03901
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
agent-identity: dev3.direct.omp.536sbpvb agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.11 agent-runtime: OMP 18.0.11 tooling-profile: dotfiles@000f2b3
agent-identity: dev3.direct.omp.536sbpvb agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.11 agent-runtime: OMP 18.0.11 tooling-profile: dotfiles@000f2b3
agent-identity: dev3.direct.omp.2cshu64q agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.9 agent-runtime: OMP 18.0.9 tooling-profile: dotfiles@b607597
d072a03 to
2b998c3
Compare
|
@codex review Posted on behalf of @schickling
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b998c3d70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let remove_count = terminal.len().saturating_sub(MAX_OBSERVE_RECEIPTS); | ||
| for (_, path) in terminal.into_iter().take(remove_count) { | ||
| if let Err(error) = fs::remove_file(&path) { |
There was a problem hiding this comment.
When a waiting client is descheduled while 256 later observations settle, this unconditional oldest-first pruning can delete its terminal receipt before ObserveClient performs the next path read. Because the corresponding request was already removed after the terminal write, the client then has neither terminal evidence nor retryable intent and eventually reports a timeout even though its observation settled. Receipt cleanup needs an acknowledgement/age policy that cannot race active waiters, rather than pruning solely by count.
Useful? React with 👍 / 👎.
| if let Some(dispatched_at) = batch.dispatched_at { | ||
| for _ in &batch.requests { | ||
| crate::metrics::record_resource_observe_settle(dispatched_at.elapsed()); | ||
| } |
There was a problem hiding this comment.
When write_receipt fails, this records resource_observe_settle_seconds before any durable result receipt exists, while the later successful retry records no replacement sample. The histogram is documented as latency through the durable receipt, so filesystem failures and retries systematically underreport settlement latency; record each request's sample only when its terminal receipt write succeeds.
Useful? React with 👍 / 👎.
1 participant
Why
Observable Resource Profiles need an exact, bounded way to request a fresh observation from the resident profile runtime. Snapshot reads alone cannot distinguish unchanged data, provider failure, stale generations, or a request that is still queued.
The protocol must not split one observation across publication and settlement frames. A split result permits partial acceptance and duplicates the existing publication path.
What
Add the supervisor-scoped ObserveNow request/receipt protocol and
st2 resource refreshCLI.Extend the Resource Profile protocol with one atomic, status-tagged
ObservationResult:unchangedfailed, with an optional diagnosticpublished, with the sharedPublicationenvelopeObservecarries the demand watermark. PeriodicPublishand demand-drivenpublishedresults share the same publication validation, snapshot acceptance, typed facts, semantic invalidation, and catch-up core. PeriodicPublishremains wire-compatible through a flattened publication envelope, so the descriptor ABI stays at version 3.How
Requests and receipts use bounded, private, atomic files under the supervisor generation scope. Admission is locked and rejects the 257th concurrent request. The supervisor retains an enqueued request until a terminal receipt is durable, recovers queued work after restart, fences each result by owner/binding/registration/watermark, coalesces one in-flight and one trailing batch per binding, and retries terminal receipt writes without redispatching settled runtime work.
The runtime result is accepted as one unit. A
publishedresult is validated and stored before the matching terminal receipt is committed. Both publication paths use the same host-computed digest and bounded facts/topics contract. Missing demand capability settles asabsentBindingwith a diagnostic instead of sending an unsupported frame.Rationale
The atomic tagged result has one source of truth for observation outcome and publication content. It removes the former two-frame settlement overlap while retaining periodic publication as an independent provider behavior.
Durable request and receipt records make demand state inspectable and restart-safe. Exact receipts preserve changed, unchanged, failed, stale, absent, unavailable, and backpressured outcomes without a correctness timer.
Verification
cargo test --features wasm-resolver --test resource_profile_supervisor_e2e -- --nocapture(7 passed)cargo test --features wasm-resolver --test agent_resource -- --nocapture(9 passed)cargo test --features wasm-resolver --lib(all workspace library tests passed)nix build .#checks.x86_64-linux.wasm-resolver-feature --no-link --print-build-logs(passed)rustfmt --checkfor every changed Rust module and integration testgit diff --checkA full
nix flake checkpassed every reached check and test suite except the pre-existingcodex_app_server::tests::a_dead_socket_is_removed_before_launchrace in one parallel derivation. The exactwasm-resolver-featureNix check then passed, including the complete Resource supervisor suite.Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile